New component: ha-expansion-panel by Misiu · Pull Request #6789 · home-assistant/frontend

Misiu · 2020-09-04T15:03:15Z

Breaking change

Proposed change

This adds a new component ha-expansion-panel. I want to use it with the time condition editor.

Supported parameters:

expanded
outlined

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New feature (thank you!)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example configuration

Additional information

This PR fixes or closes issue: fixes Add expansion panel component #6726
This PR is related to issue: Add weekday to time contidion editor #6786, Split weekday from time condition into a separate condition architecture#356
Link to documentation pull request:

Checklist

The code change is tested and works locally.
There is no commented out code in this PR.
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

src/components/ha-expansion-panel.ts

bramkragten · 2020-09-04T16:17:33Z

You should probably use a <ha-card> as basis.

Misiu · 2020-09-04T16:21:33Z

@bramkragten so I should extend ha-card?
I can try.

bramkragten · 2020-09-04T16:22:32Z

@bramkragten so I should extend ha-card?
I can try.

No, not extend, just use it in your render.

zsarnett · 2020-09-04T16:33:18Z

Should we use HA-card?

Or should this be the guts of the functionality and can be outside of a <ha-card> or can be placed with in one

@bramkragten

src/components/ha-expansion-panel.ts

zsarnett · 2020-09-09T02:50:52Z

src/components/ha-expansion-panel.ts

+      }
+
+      .summary-content {
+        margin: 12px 0;


Too much margin. Specifically on the bottom. The content is not close enough to the summary title

zsarnett · 2020-09-09T02:51:56Z

src/components/ha-expansion-panel.ts

+        line-height: 1.5;
+        letter-spacing: 0.00938em;
+        font-size: 0.9375rem;
+        font-family: "Roboto", "Helvetica", "Arial", sans-serif;


Suggested change

line-height: 1.5;

letter-spacing: 0.00938em;

font-size: 0.9375rem;

font-family: "Roboto", "Helvetica", "Arial", sans-serif;

line-height: 1.2;

text-overflow: ellipsis;

overflow: hidden;

I think ellipsis needs display, white-space an width properties, ref: https://stackoverflow.com/a/7993098/965722
I have a problem with width because it needs to be calculated.

I pulled your code and editted it. This is needed and I was able to add it.

Or you can just make it a lot and have the element using it do it :)

zsarnett · 2020-09-09T02:52:15Z

src/components/ha-expansion-panel.ts

+        margin: 12px 0;
+        display: flex;
+        flex-grow: 1;
+        transition: margin 150ms cubic-bezier(0.4, 0, 0.2, 1) 0ms;


Suggested change

transition: margin 150ms cubic-bezier(0.4, 0, 0.2, 1) 0ms;

transition: margin 150ms cubic-bezier(0.4, 0, 0.2, 1) 0ms;

overflow: hidden;

src/components/ha-expansion-panel.ts

zsarnett · 2020-09-09T02:56:12Z

src/components/ha-expansion-panel.ts

+      .summary {
+        display: flex;
+        padding: 0px 16px;
+        min-height: 48px;


actually its this that is too large that is making it not close enough to the content

src/components/ha-expansion-panel.ts

zsarnett · 2020-09-09T03:12:45Z

We should use javascript to set the max-height on the element to the scroll-height and then transition the max height instead of the height, either using CSS or Javascript and requestAnimationFrame

src/components/ha-expansion-panel.ts

zsarnett · 2020-09-09T20:54:37Z

I really like this Component. I already have a branch that is using it 😄

Misiu · 2020-09-09T22:00:40Z

@zsarnett that's why I want this to be part of HA :) just need to polish this a bit more.

zsarnett · 2020-09-19T05:55:54Z

Any update on this? Would love to be able to get this in so it can start to be used

Misiu · 2020-09-20T11:06:18Z

@zsarnett sadly no updates for now. I'll try to move things forward next week.
I'll optimize CSS and address your comments.
Not sure how to use ha-card in the code, but that will be next step.

zsarnett · 2020-09-20T14:43:47Z

I wouldn't use ha card.

But all good. Just making sure it hasn't been forgotten

single event Co-authored-by: Bram Kragten <mail@bramkragten.nl>

bramkragten · 2020-10-02T12:45:59Z

src/components/ha-expansion-panel.ts

+
+  private _handleTransitionEnd() {
+    if (this.expanded) {
+      this._container.style.height = "auto";


This works right?

Suggested change

this._container.style.height = "auto";

this._container.style.removeProperty("height");

yes, this works perfectly. I've simplified the logic a bit thanks to this.
The last thing I'd like to remove is setTimeout, but collapse doesn't work without this.

bramkragten · 2020-10-02T13:14:45Z

We can merge this as soon as we have implemented it somewhere.

Misiu · 2020-10-02T13:20:52Z

We can merge this as soon as we have implemented it somewhere.

I want to implement this in the time condition editor.
My idea is to split time and date into two panels and expand them only if something is filled.
So I'll show the time part if after or before is filled, same for the weekday section. I want to show it only if the user has changed something (selected a day).
I can create a PR to show what I have in mind.

Also, @zsarnett had usage for this component.

donkawechico · 2020-10-13T19:16:21Z

This is awesome! I also have use for it -- I want to make sequence steps collapsible and show their alias property as a short-hand description.

balloob · 2020-10-14T15:55:35Z

Why are we collapsing parts of a condition instead of the whole condition?

And when we collapse a condition, we should make sure that a summary of that condition is put in the name.

Ie "Time trigger at 10:01"

Once we have such summaries, we can just automatically collapse everything, making it easier to see what's going on in an automation.

Misiu · 2020-10-14T16:00:43Z

@balloob this PR only adds a new component. No usage. I'm planning to add usage in a separate PR.
The GIF in the first message is just to show how the component looks like.
My motivation was to split time condition into two parts, but I agree that this can be applied to all conditions.

homeassistant added the cla-signed label Sep 4, 2020

zsarnett reviewed Sep 4, 2020

View reviewed changes